Skip to content

fix(llc): null ordering for pinned_at and last_message_at channel sorts - #2889

Merged
VelikovPetar merged 3 commits into
v9from
port/master-to-v9/FLU-688_channel-sort-null-ordering
Aug 14, 2026
Merged

fix(llc): null ordering for pinned_at and last_message_at channel sorts#2889
VelikovPetar merged 3 commits into
v9from
port/master-to-v9/FLU-688_channel-sort-null-ordering

Conversation

@VelikovPetar

Copy link
Copy Markdown
Contributor

Submit a pull request

Linear: FLU-688

Github Issue: #

CLA

  • I have signed the Stream CLA (required).
  • The code changes follow best practices
  • Code changes are tested (add some information if not applicable)

Description of the pull request

Backport of master commit 91ca8b63caa7398fb7e7209fde4e7b097317863b — "fix(llc): null ordering for pinned_at and last_message_at channel sorts" (#2878).

SortOption's comparator placed NULLs using Postgres' default ordering (ASC → NULLS LAST, DESC → NULLS FIRST). The server doesn't use the default for every field — it applies explicit NULLS LAST to pinned_at and last_message_at. Since StreamChannelListController.value re-sorts every response with this comparator, those two fields ended up ordered differently than the server returned them.

Symptoms

  • [{pinned_at: -1}, {last_updated: -1}] — server returns pinned channels first; the SDK re-sorted them to the bottom.
  • {last_message_at: -1} — server puts message-less channels last; the SDK floated every empty channel to the top.

Ascending sorts were already correct, since the ascending default (nullsLast) matches the server for both fields.

Fix

nullOrdering is resolved from the field in each constructor's initializer list, so it stays a const field:

  • pinned_at, last_message_atnullsLast in both directions
  • every other field → unchanged (Postgres default)
  • an explicitly passed nullOrdering still wins

Port notes

This was a manual port, not a clean cherry-pick — v9 formats at 80 cols (short style) vs master's tall/120, and v9's SortOption has diverged from master's.

Two changes go beyond the master commit, because without them the fix is inert on v9:

  1. SortOption.fromJson — on v9 this was still the generated factory, which routes through the deprecated primary constructor and hardcodes nullsFirst, ignoring direction entirely. It is the only path PredefinedFilter.sort and the offline persisted sort (ChannelStateSortOrderConverter) go through, so porting just the constructors would have left both broken. Master gained the hand-written factory in #2709; v9's backport of that feature (#2775) did not carry it over. Ported here, with createFactory: false and sort_order.g.dart regenerated.
  2. The deprecated SortOption() constructor — removed on master by an earlier cleanup, but still live public API on v9, so it kept the bug for anyone not yet migrated to .asc/.desc. Made field-aware too.

API impact — source-compatible, patch-level:

  • nullOrdering widens from NullOrdering to NullOrdering? on the parameter only; reads stay non-nullable and there are no subclasses of SortOption in the repo.
  • _$SortOptionFromJson is library-private generated code, not public API; SortOption.fromJson's signature is unchanged.
  • Unlike master, the ported fromJson keeps the generated code's tolerant direction parse ((json['direction'] as num?)?.toInt() ?? SortOption.DESC) instead of master's hard as int cast, so payloads missing direction behave exactly as before rather than throwing.

Test instructions

cd packages/stream_chat && dart test test/src/core/api/sort_order_test.dart

Covers the field-aware defaults across both fields and directions, an explicit override beating a field-aware default, fromJson resolution (predefined filters), the missing-direction fallback, field-aware defaults on the deprecated constructor, and four ordering assertions over real ChannelState: pinned-on-top in both directions, empty channels last for last_message_at desc, and nulls-first preserved for a generic field so the general case can't silently regress.

Results: sort_order_test.dart 28/28; full stream_chat suite 1311 passed / 2 skipped; stream_chat_persistence 260 passed; stream_chat_flutter_core 233 passed. dart analyze --fatal-infos and melos run format clean.

Screenshots / Videos

No UI changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bc4155bc-4483-4868-bd28-f26dc6e3e81d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

xsahil03x added a commit that referenced this pull request Aug 14, 2026
9.27.0 is already tagged and published, so the entry belongs in a new
Unreleased section rather than an existing released one. Matches the
sibling v9 backport (#2889).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@VelikovPetar
VelikovPetar marked this pull request as ready for review August 14, 2026 13:42
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (v9@8a3ce36). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...kages/stream_chat/lib/src/core/api/sort_order.dart 72.72% 3 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##             v9    #2889   +/-   ##
=====================================
  Coverage      ?   67.32%           
=====================================
  Files         ?      431           
  Lines         ?    27410           
  Branches      ?        0           
=====================================
  Hits          ?    18454           
  Misses        ?     8956           
  Partials      ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@VelikovPetar
VelikovPetar merged commit fc3942c into v9 Aug 14, 2026
24 checks passed
@VelikovPetar
VelikovPetar deleted the port/master-to-v9/FLU-688_channel-sort-null-ordering branch August 14, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants